add support for enabling delete protection by Curverneur · Pull Request #268 · apache/cloudstack-terraform-provider

Curverneur · 2025-12-03T16:03:21Z

Description

Add support for enabling the delete protection for instances (virtual machines) and volumes (disks).

Reason

This feature allows users to enable the delete protection for certain resources to prevent accidental deletion.

References

Changes

Added deleteprotection option to cloudstack_instance and cloudstack_disk resource and updated the corresponding documentation pages.

modified cloudstack/resource_cloudstack_instance.go
modified cloudstack/resource_cloudstack_disk.go
modified website/docs/r/instance.html.markdown
modified website/docs/r/disk.html.markdown

Copilot

Pull request overview

This pull request adds delete protection support for CloudStack instances (virtual machines) and volumes (disks) by introducing a new deleteprotection boolean field to both the cloudstack_instance and cloudstack_disk resources. The feature integrates with CloudStack's API updateVirtualMachine and updateVolume endpoints to enable protection against accidental deletion.

Key Changes:

Added deleteprotection optional boolean field to instance and disk resource schemas
Implemented delete protection setting via CloudStack update APIs after resource creation
Updated documentation for both resources with usage notes about limitations when resources are managed by other services

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`cloudstack/resource_cloudstack_instance.go`	Added deleteprotection schema field and implementation to set/update protection via UpdateVirtualMachine API
`cloudstack/resource_cloudstack_disk.go`	Added deleteprotection schema field and implementation to set/update protection via UpdateVolume API
`website/docs/r/instance.html.markdown`	Documented deleteprotection parameter with usage notes and reformatted userdata_id/userdata_details entries
`website/docs/r/disk.html.markdown`	Documented deleteprotection parameter with usage notes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Curverneur · 2025-12-20T10:02:13Z

looks like some checks failed due to connection issues. could you check and trigger them again, please?

vishesh92

@Curverneur Thansk for the PR. It looks mostly good. Can you check the comments? It would be great if you can add some tests here as well.

…ction in instance and disk resource

Curverneur · 2026-03-17T15:44:27Z

Hi @vishesh92, thanks for the feedback. Changes commited as requested. Does the test meet the requirements? Do you have any suggestions for changes?

Curverneur · 2026-03-18T19:28:40Z

Hi @vishesh92, the deleteprotection parameter has been added to the api in version 4.20. Testing with older versions of cloudstack-simulator, e.g. 4.19.3, just won't configure deleteprotection for instances or disks and logs:

WARN  [c.c.a.d.ParamGenericValidationWorker] (qtp2068388745-328:ctx-e860b95a ctx-604302c4 ctx-94a7892a) (logid:8565d729) Received unknown parameters for command updateVirtualMachine. Unknown parameters : deleteprotection

Well, using ExpectError might not be the best approach as indicated in testing.go. Do you have any other ideas? How could I properly test this feature and ensure the test is successful? Had the idea to check the api version and skip the check if the version is older than 4.20, but still not sure where I could get it from.

	// ExpectError allows the construction of test cases that we expect to fail
	// with an error. The specified regexp must match against the error for the
	// test to pass.
	//
	// This functionality is only intended for provider-controlled error
	// messaging. While in certain scenarios this can also catch testing logic
	// error messages, those messages are not protected by compatibility
	// promises.
	ExpectError *regexp.Regexp

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Suppressed comments (4)

cloudstack/resource_cloudstack_instance_test.go:318

The expected-error regex ends with an unescaped . which matches any single character, not a literal period. If the actual error message ends exactly with deleted (no trailing character), this regex won’t match and the test will fail. Consider removing the trailing dot or escaping it (e.g., \\.) and/or anchoring/making the pattern more tolerant.

				// attempt to destroy vm. expected to fail due to delete protection is enabled
				Config:      fmt.Sprintf(testAccCloudStackInstance_deleteProtection, true),
				Destroy:     true,
				ExpectError: regexp.MustCompile(".*has delete protection enabled and cannot be deleted."),
			},

cloudstack/resource_cloudstack_disk_test.go:146

Same regex issue as the instance test: the unescaped trailing . matches any character, which can cause a false negative if the error string doesn’t have a trailing character. Remove the trailing dot or escape it so the test is robust.

				// attempt to destroy disk. expected to fail due to delete protection is enabled
				Config:      fmt.Sprintf(testAccCloudStackDisk_deleteProtection, true),
				Destroy:     true,
				ExpectError: regexp.MustCompile(".*has delete protection enabled and cannot be deleted."),
			},

cloudstack/resource_cloudstack_instance.go:902

Error messaging is inconsistent: Create uses "delete protection" (no underscore) while Update uses "delete_protection" (underscore). For clearer, consistent UX (and easier test matching), standardize the phrasing across create/update (prefer using the user-facing attribute name or a consistent human-readable phrase).

	// Check if the delete protection has changed and if so, update the deleteprotection
	if d.HasChange("delete_protection") {
		p := cs.VirtualMachine.NewUpdateVirtualMachineParams(d.Id())
		p.SetDeleteprotection(d.Get("delete_protection").(bool))

		_, err := cs.VirtualMachine.UpdateVirtualMachine(p)
		if err != nil {
			return fmt.Errorf(
				"Error updating the delete_protection for instance %s: %s", name, err)
		}
	}

cloudstack/resource_cloudstack_disk_test.go:157

Correct spelling in comment from 'disabledd' to 'disabled'.

				// destroy disk. expected to pass due to disk protection is disabledd
				Config:  fmt.Sprintf(testAccCloudStackDisk_deleteProtection, false),

- Read delete_protection back from the API in resourceCloudStackInstanceRead and resourceCloudStackDiskRead so the Computed attribute reflects import and out-of-band changes instead of always going stale. - Fix TestAccCloudStackInstance_deleteProtection using the disk CheckDestroy helper instead of the instance one. - Escape the trailing "." in the ExpectError regexes so they match a literal period instead of any character. - Standardize the update error message wording to "delete protection" and fix a "disabledd" typo in test comments.

# Conflicts: # cloudstack/resource_cloudstack_instance_test.go

sureshanaparti

clgtm

sudo87

Verification summary — PR #268 against the CloudStack simulator
Automated acceptance tests (ran against apache/cloudstack-simulator:4.22.1.0):
TestAccCloudStackDisk_deleteProtection — PASS (4.95s)
TestAccCloudStackInstance_deleteProtection — PASS (23.96s)

Ran scenarios against simulator

added deleteprotection option for instances and disks

2cd02a6

DaanHoogland requested review from Pearl1594, Copilot and vishesh92 and removed request for Pearl1594 and vishesh92 December 8, 2025 12:54

Copilot started reviewing on behalf of DaanHoogland December 8, 2025 12:54 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Curverneur and others added 4 commits December 16, 2025 14:20

removed commented code

d51a6cf

fix typo

255ded2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

set computed to true

b17f015

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

set computed to true

845d6e3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

vishesh92 reviewed Mar 10, 2026

View reviewed changes

Comment thread cloudstack/resource_cloudstack_disk.go Outdated

vishesh92 reviewed Mar 10, 2026

View reviewed changes

Comment thread cloudstack/resource_cloudstack_instance.go Outdated

vishesh92 reviewed Mar 10, 2026

View reviewed changes

renamed delete_protection terraform key, added tests for delete_prote…

67d670e

…ction in instance and disk resource

sureshanaparti requested a review from vishesh92 July 10, 2026 08:13

sureshanaparti added the needs-review label Jul 10, 2026

sureshanaparti added this to the v0.7.0 milestone Jul 10, 2026

sureshanaparti closed this Aug 14, 2026

sureshanaparti reopened this Aug 14, 2026

sureshanaparti requested a lite review from Copilot August 14, 2026 08:32

Copilot AI reviewed Aug 14, 2026

View reviewed changes

Comment thread cloudstack/resource_cloudstack_instance_test.go Outdated

Comment thread cloudstack/resource_cloudstack_instance.go

Comment thread cloudstack/resource_cloudstack_disk.go

Copilot started reviewing on behalf of sureshanaparti August 14, 2026 08:49 View session

sudo87 force-pushed the feature/add-resource-protection-support branch from d083011 to 67d670e Compare August 17, 2026 06:36

sureshanaparti reviewed Aug 17, 2026

View reviewed changes

Comment thread cloudstack/resource_cloudstack_disk.go

Merge remote-tracking branch 'origin/main' into HEAD

cbdabbb

# Conflicts: # cloudstack/resource_cloudstack_instance_test.go

sureshanaparti approved these changes Aug 17, 2026

View reviewed changes

sureshanaparti added status:ready-for-testing and removed needs-review labels Aug 17, 2026

sureshanaparti assigned kiranchavala and unassigned kiranchavala Aug 17, 2026

sureshanaparti added the improvement label Aug 17, 2026

sudo87 approved these changes Aug 17, 2026

View reviewed changes

sudo87 merged commit f8e0ca8 into apache:main Aug 17, 2026
16 checks passed

Uh oh!

Conversation

Curverneur commented Dec 3, 2025

Description

Reason

References

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Curverneur commented Dec 20, 2025

Uh oh!

Uh oh!

Uh oh!

vishesh92 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Curverneur commented Mar 17, 2026

Uh oh!

Curverneur commented Mar 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

sudo87 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vishesh92 left a comment •

edited

Loading